Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-implement cantus index input tool #1538

Merged
merged 15 commits into from
Jun 18, 2024

Conversation

lucasmarchd01
Copy link
Contributor

@lucasmarchd01 lucasmarchd01 commented Jun 17, 2024

This PR re-implements the Cantus Index search view on the chant create page. This feature was turned off in #1142, and many users have been requesting the feature again.

The previous implementation parsed HTML from https://cantusindex.org/search API. Now, we use https://cantusindex.org/json-text/. This API is still not available on cantusindex.uwaterloo.ca, so we will have to switch over once it is.

We still have to write tests for this view, so I will convert that to a new issue to get this up and running sooner.

Chant Create:
Screenshot 2024-06-17 at 12 26 54 PM
CISearchView:
Screenshot 2024-06-17 at 12 27 28 PM

Resolves #1391

@dchiller
Copy link
Contributor

I find some things weird about the ci_search template, but am assuming you just re-added what we had before?

@lucasmarchd01
Copy link
Contributor Author

I find some things weird about the ci_search template, but am assuming you just re-added what we had before?

Yeah, I thought about making it look more like our other pages by adding base.html and so on but I think it was made this way to save space and scrolling time. The template is exactly the same as before but with some added javascript to handle populating the django-autocomplete-light selector for genre.

@lucasmarchd01
Copy link
Contributor Author

Also @dchiller (or @annamorphism ) check out #1391 (comment) just to make sure this is how we'd like to implement this.

@dchiller
Copy link
Contributor

dchiller commented Jun 18, 2024

Also @dchiller (or @annamorphism ) check out #1391 (comment) just to make sure this is how we'd like to implement this.

I think this is good for now ... If we figure out a way to increase the number of results in future then we make that a separate issue, but at least this feature will be available...

I do think the current message might therefore be a little misleading, so would agree about changing it to be something like the other option you suggested.

@lucasmarchd01 lucasmarchd01 merged commit 6da51fa into DDMAL:develop Jun 18, 2024
1 check passed
@dchiller dchiller mentioned this pull request Jul 12, 2024
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We need to reimplement the CI Search View
2 participants